Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

errors: return all token fetch related errors as structured #380

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xaurx
Copy link

@xaurx xaurx commented Apr 19, 2019

Right now all non-HTTP errors are returned as plain error with combined
text messages into single string, e.g.:
oauth2: cannot fetch token: Post https://oauth2.googleapis.com/token: read tcp 10.0.6.7:46650->172.217.212.95:443: read: connection reset by peer

So caller doesn't have a chance to check easily if underlying error was
retriable (e.g. net.IsTemporary() or DNS error) or not.

This patch wraps ALL errors as RetrieveError with additional Err field
in case it was non-http error.

Right now all non-HTTP errors are returned as plain error with combined
text messages into single string, e.g.:
oauth2: cannot fetch token: Post https://oauth2.googleapis.com/token: read tcp 10.0.6.7:46650->172.217.212.95:443: read: connection reset by peer

So caller doesn't have a chance to check easily if underlying error was
retriable (e.g. net.IsTemporary() or DNS error) or not.

This patch wraps ALL errors as RetrieveError with additional Err field
in case it was non-http error.
@gopherbot
Copy link
Contributor

This PR (HEAD: 6b30711) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/oauth2/+/172877 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@tbonfort
Copy link

tbonfort commented Nov 4, 2020

Being able to inspect errors to determine their temporary state is an essential need in order to build resilient systems, and currently this issue is preventing this. Would the maintainers of this repo accept a modified version of this patch with RetrieveError implementing an Unwrap method so as to support go 1.13 error chaining? Another option would be to bump the go.mod go version to 1.13 and use fmt.Errorf's %w instead of %v

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants